Skip to content

[2025-06-lwg-21] P3149R11 async_scope - Creating scopes for non-sequential concurrency #8027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 16, 2025

Conversation

burblebee
Copy link
Contributor

@burblebee burblebee commented Jun 27, 2025

[exec.associate]p12 Renamed "scope_token" to "scope_tkn" due to libconcept of
the same name.
[exec.spawn.future]p2 [exec.spawn]p2 Employ bullets. [exec.stop.when]p3 [exec.counting.scope]p11 Merged paragraphs via bullets for
clarification ("sndr" and "token" aren't defined otherwise and get confused
with the \exposids of the same name).
[exec.counting.scopes.general] Added missing \expos comments. [exec.counting.scopes.general] Fixed "close" in scope-state-type to "closed". [exec.counting.scopes.general]p1.3 Changed "the scope" to "scope".

Fixes #7958.
Fixes cplusplus/papers#1800

@jensmaurer
Copy link
Member

[exec.associate]p12 Renamed "scope_token" to "scope_tkn" due to libconcept of the same name.

That's not really a hard conflict due to name lookup rules, right?

@burblebee burblebee force-pushed the motions-2025-06-lwg-21 branch from ed9536d to 109df4b Compare June 30, 2025 23:31
@burblebee
Copy link
Contributor Author

[exec.associate]p12 Renamed "scope_token" to "scope_tkn" due to libconcept of the same name.

That's not really a hard conflict due to name lookup rules, right?

check-output.sh won't allow it.

@burblebee burblebee marked this pull request as ready for review July 1, 2025 01:36

auto query(execution::get_scheduler_t) const noexcept {
return @\exposid{loop}@->get_scheduler();
\indexlibraryglobal{execution::\exposid{associate-data}}%
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge, we don't index exposition-only declarations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we didn't used to but we do now (probably since more and more of them are being introduced in the library?)

Comment on lines +5600 to +5904
auto token = std::move(this->@\exposid{token}@);

@\exposid{destroy}@();
token.disassociate();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The paper has a blank line between each statement. I think we should either do what the paper says, or just get rid of all. The blank line after auto token is not really necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting of codeblocks is up to the discretion of the editor. I typically try to be consistent with surrounding code, which in this case tends to insert a space after declarations when there is more than 1 statement to follow. Feel free to submit a PR to format this differently.

@Eisenwave
Copy link
Member

I think I've missed a few \linebreak -> \brk chnages; anyhow, all of those should be changed.

@burblebee burblebee requested a review from Eisenwave July 15, 2025 06:32
Copy link
Member

@Eisenwave Eisenwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some loose threads (like FWD-ENV-T, _t in an exposition-only name, indexing for exposition-only names), but I don't really know how to go about those, so I'll see myself out of the review process.

Thanks for the other changes in the latest push; those look good.

Maybe Thomas will know what to do once he gets here.

@Eisenwave Eisenwave dismissed their stale review July 15, 2025 17:42

Direction unclear, feedback from Thomas or Jonathan needed.

@tkoeppe tkoeppe force-pushed the motions-2025-06-lwg-21 branch from fbe7a4f to 4b298cb Compare July 16, 2025 00:10
[exec.associate]p12 Renamed "scope_token" to "scope_tkn" due to libconcept of
  the same name.
[exec.spawn.future]p2 [exec.spawn]p2 Employ bullets.
[exec.stop.when]p3 [exec.counting.scope]p11 Merged paragraphs via bullets for
  clarification ("sndr" and "token" aren't defined otherwise and get confused
  with the \exposids of the same name).
[exec.counting.scopes.general] Added missing \expos comments.
[exec.counting.scopes.general] Fixed "close" in scope-state-type to "closed".
[exec.counting.scopes.general]p1.3 Changed "the scope" to "scope".
@tkoeppe tkoeppe force-pushed the motions-2025-06-lwg-21 branch from 4b298cb to 6ab1d5d Compare July 16, 2025 11:41
@tkoeppe tkoeppe force-pushed the motions-2025-06-lwg-21 branch from 6ab1d5d to b050f96 Compare July 16, 2025 12:19
@tkoeppe tkoeppe merged commit db62123 into main Jul 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants